Skip to content

Conversation

@mhegazy
Copy link
Contributor

@mhegazy mhegazy commented Nov 18, 2016

A few changes:

  • avoid using this types and instead add the overloads to every derived interface
  • use Mapped types instead of listing event names/tag names explicitly in overloads

//cc: @zhengbli

Build diagnostics building dom.genenrated.d.ts before the change:

Files:            2
Lines:        18814
Nodes:       102909
Identifiers:  35546
Symbols:      95889
Types:        12369
Memory used: 94151K
I/O read:     0.00s
I/O write:    0.00s
Parse time:   0.36s
Bind time:    0.18s
Check time:   1.13s
Emit time:    0.02s
Total time:   1.68s

After the change:

Files:            2
Lines:        17559
Nodes:        69571
Identifiers:  23524
Symbols:      18183
Types:         4095
Memory used: 52984K
I/O read:     0.01s
I/O write:    0.00s
Parse time:   0.32s
Bind time:    0.15s
Check time:   0.50s
Emit time:    0.02s
Total time:   0.98s

@rictic
Copy link
Contributor

rictic commented Nov 19, 2016

This is so cool! It looks like this will be great news for custom elements. e.g. experimenting with the nightly build and some of the example code in microsoft/TypeScript#12311 it seems like this typechecks:

class MySlider extends HTMLElement {
  slide() {}
}
customElements.define('my-slider', MySlider);
interface HTMLKind {
  'my-slider': MySlider;
}

document.createElement('my-slider').slide()

So awesome!

[ for i' in implementis do
yield! match i' with
| Some i -> GetEventHandler i
| Some i -> if hasHandler i then [i] else []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this returns the interface itself instead of the handlers it has?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. i could make it the interface name. but seemed the same

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then the name of the variable should be changed to implementedInterface and other places, otherwise it can be confusing later


let EmitElementTagNameMap () =
Pt.printl "interface ElementTagNameMap {"
Pt.increaseIndent()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like ElementTagNameMap interface is always the same as HTMLElementTagNameMap? Then make is an alias instead

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no they are different. not sure why though. i wanted to ask you about that.

Copy link
Contributor

@zhengbli zhengbli Nov 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha now I remember, the EmitHTMLElementTagNameMap only prints tag elements that either extends or implements the HTMLElement type.

Pt.printl "interface ElementListTagNameMap {"
Pt.increaseIndent()
for e in tagNameToEleName do
Pt.printl "\"%s\": NodeListOf<%s>;" (e.Key.ToLower()) e.Value
Copy link
Contributor

@zhengbli zhengbli Nov 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do

type ElementListTagNameMap = { [K in ElementTagNameMap]: NodeListOf<ElementTagNameMap[K]> }

and avoid the loop

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is what i had earlier, then changed it. the issue is that NodeListOf has a constraint that T extends Node. we can not verify this constraint on ElementTagNameMap[K] unless i add a string indexer. adding the indexer makes the language service experience of the signature help much worse. so this is a work around for that.

TS.fsx Outdated
Pt.print " {"
Pt.increaseIndent()
ownEventHandles |> List.iter EmitInterfaceEventMapEntry
// Pt.printl "[x: string]: Event;"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

@zhengbli
Copy link
Contributor

The addEventListener in the addedTypes.json file need to be changed as well, and other than that looks good to me. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants